Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

gcc: patches: reorganize and simplify #250496

Merged
merged 2 commits into from Sep 5, 2023
Merged

gcc: patches: reorganize and simplify #250496

merged 2 commits into from Sep 5, 2023

Conversation

ghost
Copy link

@ghost ghost commented Aug 21, 2023

Description of changes

This PR sorts the huge pile of patches and deduplicates it. Unlike its predecessors #249707 and #250489 this commit does affect eval.

Patches below are organized into three general categories:

  1. Patches relevant to gcc>=12 on every platform
  2. Patches relevant to gcc>=12 on specific platforms
  3. Patches relevant only to gcc<12, sorted by the newest version to which they might be relevant

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 23.11 Release Notes (or backporting 23.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

@ghost ghost mentioned this pull request Aug 21, 2023
Merged
1 task
gcc: patches: reorganize and simplify
62c676c 
This PR sorts the huge pile of patches and deduplicates it.  Unlike
its predecessors #249707 and
#250489 this commit *does*
affect eval.

Patches below are organized into three general categories:

1. Patches relevant to gcc>=12 on every platform
2. Patches relevant to gcc>=12 on specific platforms
3. Patches relevant only to gcc<12, sorted by the newest version to
   which they might be relevant
@ghost ghost marked this pull request as ready for review August 21, 2023 09:18
@ghost ghost requested a review from matthewbauer as a code owner August 21, 2023 09:18
@Artturin
Copy link
Member

Artturin commented Sep 2, 2023 •
edited
Loading

when i apply

diff --git a/pkgs/development/compilers/gcc/patches/default.nix b/pkgs/development/compilers/gcc/patches/default.nix
index f65842196ab1..1d010600c0d4 100644
--- a/pkgs/development/compilers/gcc/patches/default.nix
+++ b/pkgs/development/compilers/gcc/patches/default.nix
@@ -70,8 +70,8 @@ in
     "9"  = [ ./no-sys-dirs-riscv-gcc9.patch ];
   }."${majorVersion}" or [])
 )
-++ optional (atLeast12 && langAda) ./gnat-cflags-11.patch
-++ optional langFortran (if atLeast12 then ./gcc-12-gfortran-driving.patch else ./gfortran-driving.patch)
+++ optional (atLeast12 && true) ./gnat-cflags-11.patch
+++ optional true (if atLeast12 then ./gcc-12-gfortran-driving.patch else ./gfortran-driving.patch)
 ++ optional atLeast7 ./ppc-musl.patch
 ++ optional is12 ./12/lambda-ICE-PR109241.patch # backport ICE fix on ccache code
 ++ optional (atLeast9 && langD) ./libphobos.patch

there are no hash changes on x86_64-linux or pkgsCross.aarch64-multiplatform.gcc

i pushed a commit so we can check all 4 systems

@Artturin
Copy link
Member

Artturin commented Sep 2, 2023 •
edited
Loading

checked locally and

     32 x86_64-darwin
    810 x86_64-linux

it's stuff like this

/nix/store/x3jz2vngdn4777kghgffnb6czal6lyrg-gcc-9.5.0.drv:{info,lib,man,out}
The set of input source names do not match:
  + gfortran-driving.patch
  + no-sys-dirs-riscv-gcc9.patch
  + ppc-musl.patch

/nix/store/8zrmfp1zvm0l6a60648rk6l0x68ii7fl-gcc-10.5.0.drv:{info,lib,man,out}
The set of input source names do not match:
  + gfortran-driving.patch
  + ppc-musl.patch

@Artturin
Copy link
Member

Artturin commented Sep 2, 2023 •
edited
Loading

gcc49.cc fails with

gcc> applying patch /nix/store/bqdcpm73hcngcsk8081ray23fznlah0h-libsanitizer-no-cyclades-9.patch
gcc> patching file libsanitizer/sanitizer_common/sanitizer_common_interceptors_ioctl.inc
gcc> Hunk #1 succeeded at 354 (offset -11 lines).
gcc> patching file libsanitizer/sanitizer_common/sanitizer_platform_limits_posix.cc
gcc> Hunk #1 FAILED at 157.
gcc> Hunk #2 succeeded at 297 with fuzz 2 (offset -169 lines).
gcc> Hunk #3 succeeded at 654 with fuzz 2 (offset -178 lines).
gcc> 1 out of 3 hunks FAILED -- saving rejects to file libsanitizer/sanitizer_common/sanitizer_platform_limits_posix.cc.rej

applied

diff --git a/pkgs/development/compilers/gcc/patches/default.nix b/pkgs/development/compilers/gcc/patches/default.nix
index 1d010600c0d4..26730dd89246 100644
--- a/pkgs/development/compilers/gcc/patches/default.nix
+++ b/pkgs/development/compilers/gcc/patches/default.nix
@@ -71,7 +71,7 @@ in
   }."${majorVersion}" or [])
 )
 ++ optional (atLeast12 && true) ./gnat-cflags-11.patch
-++ optional true (if atLeast12 then ./gcc-12-gfortran-driving.patch else ./gfortran-driving.patch)
+++ optional (!is49 && true) (if atLeast12 then ./gcc-12-gfortran-driving.patch else ./gfortran-driving.patch)
 ++ optional atLeast7 ./ppc-musl.patch
 ++ optional is12 ./12/lambda-ICE-PR109241.patch # backport ICE fix on ccache code
 ++ optional (atLeast9 && langD) ./libphobos.patch

to get a cleaner diff

image

A ordering issue?

@Artturin
Copy link
Member

Artturin commented Sep 2, 2023

Once I move ++ optional (atLeast49 && !atLeast9) ./libsanitizer-no-cyclades-9.patch to line 273 the patch applies

@Artturin
Copy link
Member

Artturin commented Sep 2, 2023 •
edited
Loading

Conflict comes from

(fetchpatch {
  name = "avoid-ustat-glibc-2.28.patch";
  url = "https://gitweb.gentoo.org/proj/gcc-patches.git/plain/4.9.4/gentoo/100_all_avoid-ustat-glibc-2.28.patch?id=55fcb515620a8f7d3bb77eba938aa0fcf0d67c96";
  sha256 = "0b32sb4psv5lq0ij9fwhi1b4pjbwdjnv24nqprsk14dsc6xmi1g0";
})

@Artturin
Copy link
Member

Artturin commented Sep 2, 2023

Works on x86_64-linux
nix build -f . "gcc49.cc" "gcc6" "gcc7" "gcc8" "gcc9" "gcc10" "gcc11" "gcc12" "gcc13" "gnat" "gcc_multi" "gcc_debug" "gcc-arm-embedded"

@Artturin
Copy link
Member

Artturin commented Sep 2, 2023 

$ export ATTR="legacyPackages.x86_64-darwin.gcc11" && nix-diff $(nix eval --raw ".?rev=$(git merge-base master HEAD)#${ATTR}.drvPath") $(nix eval --raw ".#${ATTR}.drvPath")                                                    - /nix/store/l9iandxk0hr73jgn6bzg0xd59kryiryn-gcc-wrapper-11.4.0.drv:{out}
+ /nix/store/802gh7hc6ppny98pf2rxd98fb5kcvgr0-gcc-wrapper-11.4.0.drv:{out}
• The input derivation named `gcc-11.4.0` differs
  - /nix/store/lk2a3giq4jgf5rkbyhscpi6vy1ydmgbm-gcc-11.4.0.drv:{info,lib,man,out}
  + /nix/store/gdqq0iymr5cmpqykm3iamjs280zir581-gcc-11.4.0.drv:{info,lib,man,out}
  • The set of input source names do not match:
      + no-sys-dirs-riscv.patch
      + ppc-musl.patch
  • The set of input derivation names do not match:
      - ff4bf326d03e750a8d4905ea49425fe7d15a04b8..gcc-11.4-darwin-r0.diff
  • The environments do not match:
      patches=''
      /nix/store/8bl27bnyrs5s6hw33xvbh5msxh5zsqjy-fix-bug-80431.patch /nix/store/pz03lhak99nzyyni72m14zsjzfjvqvr8-no-sys-dirs.patch /nix/store/m4q8mcyx12nr1dabsk2z88g8nnqg5j51-ff4bf326d03e750a8d4905ea49425fe7d15a04b8..gcc-11.4-darwin-r0.diff/nix/store/k346ih554hrnzq136jps90xq7v11z3vc-no-sys-dirs-riscv.patch /nix/store/632b0y5mkcdwbsw2g3xh5qznw2vv5axr-ppc-musl.patch /nix/store/fcs94pxlllbjfb3d2p4sf2rxpiyv1is7-gcc-issue-103910.patch
  ''

the removal of darwin-r0.diff patch from gcc11 maybe was intentional.

Can't test darwin builds because the community builder says "error: writing to file: No space left on device"

Can someone with darwin run
nix build ".#gcc11" --system x86_64-darwin

@Artturin Artturin merged commit 161bf63 into NixOS:staging Sep 5, 2023
25 checks passed
@ghost ghost deleted the simplify branch October 22, 2023 11:52
@Artturin Artturin mentioned this pull request Jan 29, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@matthewbauer matthewbauer Awaiting requested review from matthewbauer

Assignees
No one assigned
Labels
6.topic: hygiene 8.has: clean-up 10.rebuild-darwin: 501+ 10.rebuild-darwin: 5001+ 10.rebuild-linux: 501+ 10.rebuild-linux: 5001+ 10.rebuild-linux-stdenv
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@Artturin